Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: refactor resources/data to use the new serialization/deserializ… #381

Merged
merged 2 commits into from
May 17, 2022

Conversation

TomerHeber
Copy link
Collaborator

…ation logic

The code is going through refactoring. This is just one of many PRs.

Solution

Added "-" handling to tfschema (updated DEVELOPMENT.md).
Some refactoring of serialization. (Much more to do).

Copy link
Contributor

@razbensimon razbensimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomerHeber generally looks good to me.
But I am not familiar with tfschema so I am not sure what the - actually means.
It is something supported by it? Or it is "customer syntax" you have added?
Because I see that on the read/write methods it needs a spacial reference.

Maybe can you explain more about it and the goal here?

@TomerHeber
Copy link
Collaborator Author

@TomerHeber generally looks good to me. But I am not familiar with tfschema so I am not sure what the - actually means. It is something supported by it? Or it is "customer syntax" you have added? Because I see that on the read/write methods it needs a spacial reference.

Maybe can you explain more about it and the goal here?

@razbensimon I also added an explanation to DEVELOPMENT.md.

The writeResourceData (and readReasourceData) is a utility function that reads (writes) all the fields in (to) the interface and updates (reads) the schema via set (get) calls.
This is future proof: in case you add more fields to the schema/api - no additional coding is needed (other than updating the schema).

In general, if a struct field is called "EnvironmentId" it will be called in the schema "environment_id".
tfschema is a tag (created by us) used by writeResourceData and readReasourceData, to override this behavior. (such as overriding the default CamalCase to snake_case conversion). E.g. convert "EnvironmentId" to "my_special_environment_id".

Just as json:"-" omits fields. I added the same option to tfschema. Some fields we may still want to handle manually and avoid using writeResourceData and readResourceData.

Copy link
Contributor

@razbensimon razbensimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation 👍

@github-actions github-actions bot added the ready to merge PR approved - can be merged once the PR owner is ready label May 17, 2022
@TomerHeber TomerHeber merged commit 887579d into main May 17, 2022
@TomerHeber TomerHeber deleted the feat-refactor-serialization-#376 branch May 17, 2022 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature pending final review provider ready to merge PR approved - can be merged once the PR owner is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants